fix: check target run capabilities before encrypting hook payloads#1572
Open
TooTallNate wants to merge 1 commit intomainfrom
Open
fix: check target run capabilities before encrypting hook payloads#1572TooTallNate wants to merge 1 commit intomainfrom
TooTallNate wants to merge 1 commit intomainfrom
Conversation
🦋 Changeset detectedLatest commit: d251cd6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
Contributor
🧪 E2E Test Results❌ Some tests failed Summary
❌ Failed Tests🌍 Community Worlds (59 failed)mongodb (2 failed):
redis (2 failed):
turso (55 failed):
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
❌ 🌍 Community Worlds
✅ 📋 Other
❌ Some E2E test jobs failed:
Check the workflow run for details. |
Contributor
📊 Benchmark Results
workflow with no steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro workflow with 1 step💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express workflow with 10 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 25 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 50 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Promise.all with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro Promise.all with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express | Nitro Promise.all with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Promise.race with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Promise.race with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Promise.race with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 10 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro workflow with 25 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express | Nitro workflow with 50 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 10 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 25 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 50 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Stream Benchmarks (includes TTFB metrics)workflow with stream💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) stream pipeline with 5 transform steps (1MB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) 10 parallel streams (1MB each)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express fan-out fan-in 10 streams (1MB each)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro SummaryFastest Framework by WorldWinner determined by most benchmark wins
Fastest World by FrameworkWinner determined by most benchmark wins
Column Definitions
Worlds:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds backward-compatible hook/webhook resume behavior by disabling payload encryption when the target workflow run was created by an older @workflow/core deployment that can’t decode the encr serialization format.
Changes:
- Introduces
getRunCapabilities()(version-based capabilities) and uses it inresumeHook()to suppress encryption for pre-4.2.0-beta.64runs. - Adds unit tests for capability detection across version ranges.
- Adds
semverto the pnpm catalog and updates workspace packages/lockfile to consume it viacatalog:.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-workspace.yaml | Adds semver to the shared version catalog. |
| pnpm-lock.yaml | Updates lockfile for semver and related type deps/catalog usage. |
| packages/next/package.json | Switches semver dependency to catalog:. |
| packages/core/package.json | Adds semver (+ @types/semver) to support runtime capability checks. |
| packages/core/src/runtime/resume-hook.ts | Checks target run capabilities before encrypting dehydrated hook payloads. |
| packages/core/src/capabilities.ts | Implements capability lookup based on workflowCoreVersion. |
| packages/core/src/capabilities.test.ts | Tests version threshold behavior for encryption support. |
| .changeset/fix-hook-resume-encryption-compat.md | Publishes a patch changeset documenting the compatibility fix. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+131
to
+136
| const capabilities = getRunCapabilities( | ||
| workflowRun.executionContext?.workflowCoreVersion | ||
| ); | ||
| if (!capabilities.supportsEncryption) { | ||
| encryptionKey = undefined; | ||
| } |
There was a problem hiding this comment.
workflowRun.executionContext is typed as Record<string, any> | undefined (from @workflow/world), so workflowCoreVersion may be non-string. Passing a non-string (or malformed string) into getRunCapabilities() risks a runtime throw inside semver comparison. Consider normalizing here (e.g., only pass through when it’s a string) so resumeHook never fails due to unexpected executionContext contents.
packages/core/src/capabilities.ts
Outdated
Comment on lines
+43
to
+44
| return { | ||
| supportsEncryption: semver.gte(workflowCoreVersion, ENCRYPTION_MIN_VERSION), |
There was a problem hiding this comment.
semver.gte(workflowCoreVersion, ENCRYPTION_MIN_VERSION) will throw if workflowCoreVersion is not a valid semver string. Since this value ultimately comes from stored run metadata (and may be missing/malformed/user-supplied), it would be safer to treat invalid versions as “no encryption” instead of throwing (e.g., validate with semver.valid / semver.clean, or wrap the comparison in a try/catch and return supportsEncryption: false on errors).
Suggested change
| return { | |
| supportsEncryption: semver.gte(workflowCoreVersion, ENCRYPTION_MIN_VERSION), | |
| const validVersion = semver.valid(workflowCoreVersion); | |
| if (!validVersion) { | |
| return { supportsEncryption: false }; | |
| } | |
| return { | |
| supportsEncryption: semver.gte(validVersion, ENCRYPTION_MIN_VERSION), |
Comment on lines
+4
to
+14
| describe('getRunCapabilities', () => { | ||
| it('returns no encryption for undefined version', () => { | ||
| const caps = getRunCapabilities(undefined); | ||
| expect(caps.supportsEncryption).toBe(false); | ||
| }); | ||
|
|
||
| it('returns no encryption for pre-encryption versions', () => { | ||
| expect(getRunCapabilities('4.1.0-beta.63').supportsEncryption).toBe(false); | ||
| expect(getRunCapabilities('4.0.1-beta.27').supportsEncryption).toBe(false); | ||
| expect(getRunCapabilities('3.0.0').supportsEncryption).toBe(false); | ||
| }); |
There was a problem hiding this comment.
Tests cover expected version ordering, but they don’t assert behavior for malformed/unknown workflowCoreVersion values (e.g. 'dev', 'v4.2.0-beta.64', or other non-semver strings). Given the runtime value originates from persisted metadata, please add a test that getRunCapabilities() returns supportsEncryption=false and does not throw for invalid version strings.
When resumeHook()/resumeWebhook() is called on a newer deployment that supports encryption, it would encode the payload with the 'encr' format. If the target workflow run was created by an older deployment that predates encryption support, the run would fail with: Error: Unknown serialization format: "encr". Known formats: devl Add a capabilities table that maps @workflow/core versions to supported serialization formats. Before encoding, resumeHook() now checks the target run's workflowCoreVersion and suppresses encryption when the run's deployment doesn't support it.
TooTallNate
force-pushed
the
fix/hook-resume-encryption-compat
branch
from
389467d to
d251cd6
Compare
pranaygp
approved these changes
Apr 1, 2026
Collaborator
pranaygp
left a comment
pranaygp
left a comment
There was a problem hiding this comment.
Looks good to me. The compatibility gate in resumeHook() matches the rollout boundary for encrypted serialization and the added capability tests cover the key version cases.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
resumeHook()/resumeWebhook()failing withUnknown serialization format: "encr"when the target workflow run was created by a pre-encryption deploymentpackages/core/src/capabilities.ts) that maps@workflow/coreversions to supported serialization formats, using thesemverpackage for version comparisonresumeHook()now checks the target run'sworkflowCoreVersion(fromexecutionContext) and suppresses encryption when the run's deployment predates theencrformat (introduced in4.2.0-beta.64)How it works
Three data formats exist in the wild:
specVersion === 1(legacy)isLegacySpecVersion()devlbinaryspecVersion === 2,workflowCoreVersion < 4.2.0-beta.64encr+devlspecVersion === 2,workflowCoreVersion >= 4.2.0-beta.64When
workflowCoreVersionisundefined(very old runs that predate the field), we assume the most conservative capabilities (no encryption).Additional changes
semverto the pnpm workspace catalog and updated@workflow/nextto usecatalog:instead of a hardcoded version